Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement contactListMapFromJson() and contactListMapToJson() methods #316

Merged
merged 10 commits into from
Jun 7, 2021

Conversation

GiulioRomualdi
Copy link
Member

@GiulioRomualdi GiulioRomualdi commented May 27, 2021

This PR closes #315 in detail I used nlohmann json to parse the JSON file.

The two functions can be used to store and read from a JSON file the contact lists.

TODO

  • write the tests
  • Update the CI installing nlohmann json (At least 3.7.3 is required - the one provided by apt)

@GiulioRomualdi GiulioRomualdi added the enhancement New feature or request label May 27, 2021
@GiulioRomualdi GiulioRomualdi self-assigned this May 27, 2021
@GiulioRomualdi GiulioRomualdi force-pushed the feature/contact_parser branch 2 times, most recently from 9f3ca6c to 83225a1 Compare May 27, 2021 21:36
@GiulioRomualdi
Copy link
Member Author

GiulioRomualdi commented May 27, 2021

@traversaro before merging (or just after) the PR we should add:

@GiulioRomualdi
Copy link
Member Author

@S-Dafarra if you agree I can merge the PR

Copy link
Member

@S-Dafarra S-Dafarra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I somehow lost this

* Parse a ContactListMap from a JSON file.
* @param filename the name of the file that should be loaded.
* @return return a ContactListMap containing all the contacts.
* @note The JSON file should have the following structure
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @note The JSON file should have the following structure
* @note The JSON file should have a structure similar to the following example.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it would be nice to comment somewhere why the index is always -1. For example, you could mention that those parameters are the same of Contact. Also, left and right should resemble more a custom name, like my_left_foot.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I did some modifications f14c9d6

what do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

Copy link
Contributor

@paolo-viceconte paolo-viceconte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good to me.

As a doublecheck, I also tried to store a sequence of footsteps from hadherent through contact_list_map_to_json(), which then @GiulioRomualdi was able to read.

@traversaro traversaro removed their request for review June 7, 2021 09:43
@GiulioRomualdi GiulioRomualdi merged commit 6381938 into master Jun 7, 2021
@GiulioRomualdi GiulioRomualdi deleted the feature/contact_parser branch June 7, 2021 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement a class that reads the planned footsteps from file
3 participants